fix: use MPI_STATUS_SIZE=6 for Open MPI (avoids 4-byte stack overflow) - #137
Open
HarshitaKalani wants to merge 1 commit into
Open
fix: use MPI_STATUS_SIZE=6 for Open MPI (avoids 4-byte stack overflow)#137HarshitaKalani wants to merge 1 commit into
HarshitaKalani wants to merge 1 commit into
Conversation
Open MPI 5.x defines `sizeof(MPI_Status) = 24` bytes (6 ints on typical LP64 platforms), whereas MPICH and older Open MPI releases used a 5-int (20-byte) layout. The Fortran binding hardcoded `MPI_STATUS_SIZE = 5`, so when calling into the native MPI_Recv (and similar) with the fixed-size `integer, dimension(MPI_STATUS_SIZE) :: tmp_status` buffer, the C runtime would write past the end of the buffer and corrupt the surrounding stack. Depending on the caller's stack layout this could manifest as a hard segmentation fault deep inside the caller (e.g. inside a subsequent `_lcompilers_string_format_fortran`) at an address of the form `0x7fff00000000` — the upper half of a stack pointer overwritten by the 4-byte overflow. Gate `MPI_STATUS_SIZE` on the existing `-DOPEN_MPI` preprocessor symbol (already used elsewhere in mpi_c_bindings.f90) so it matches the underlying C header: * Open MPI -> 6 * MPICH -> 5 (unchanged) This is caught by `tests/recv_1.f90`, which crashed on Linux/x86_64 with Open MPI 5. Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MPI_Status, but the Fortran binding hardcodedMPI_STATUS_SIZE = 5. This causes the nativeMPI_Recv(and any other status-writing call) to write 4 bytes past the end of the Fortrantmp_statusbuffer allocated inside our wrappers, corrupting the caller's stack._lcompilers_string_format_fortran) at a suspicious address such as0x7fff00000000— the upper half of a stack pointer left over after the 4-byte overflow.MPI_STATUS_SIZEon the existing-DOPEN_MPIpreprocessor symbol (already used elsewhere inmpi_c_bindings.f90) so it matches the underlying C header:Rationale
Verified against Open MPI 5.0.10 that
sizeof(MPI_Status) == 24and the Fortran binding shipped bympi.modexposesMPI_STATUS_SIZE = 6. Our own binding must agree, otherwise we hand C a smaller buffer than it expects.Verification
tests/recv_1.f90crashes withSegmentation fault: address not mapped to object at address 0x7fff00000000insideprint_into_stringof the LFortran runtime.tests/run_tests.shsuite passes (57/57 test-rank combinations, includingrecv_1) at the currently pinned upstream commit31033d3using LFortran built with LLVM 15 on Linux/x86_64 with Open MPI 5.0.10.Test plan
FC="lfortran --cpp" ./tests/run_tests.shpasses end-to-end with Open MPI 5.Made with Cursor